feat(p2p): RLPx IPv6 dual-stack TCP binding and fix DiscV5 ENR tcp/tcp6 port fields#9873
Merged
usmansaleem merged 80 commits intobesu-eth:mainfrom Feb 24, 2026
Merged
Conversation
Enable dual-stack discovery by accepting one IPv4 + one IPv6 address via --p2p-host and --p2p-interface (arity 1..2), with a dedicated --p2p-port-ipv6 (default 30404). When configured, the ENR includes ip6/tcp6/udp6 fields and the discovery system listens on both address families. Inbound peers with IPv6 ENR records are resolved using their v6 address when the local node supports dual-stack. Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Usman Saleem <usman@usmans.info>
When only an IPv6 address is provided for --p2p-host, populate ip6/tcp6/udp6 ENR fields instead of ip/tcp/udp. The ENR field selection is now based on the actual address type rather than always assuming IPv4 for the primary address. Signed-off-by: Usman Saleem <usman@usmans.info>
Replace the 6-parameter initializeLocalNode overload with a single method accepting HostEndpoint and Optional<HostEndpoint>, making the API cleaner and the optional nature of IPv6 explicit at the type level. Signed-off-by: Usman Saleem <usman@usmans.info>
NAT services (UPnP, NAT-PMP) only support IPv4. Skip NAT resolution for IPv6 addresses as they are typically globally routable and don't require NAT traversal. Signed-off-by: Usman Saleem <usman@usmans.info>
Move the IPv6 endpoint field addition inside the primaryIsIpv4 check to ensure IPv6 fields are only added when there's a genuine dual-stack setup (IPv4 primary + IPv6 secondary). This prevents IPv6 fields from being incorrectly overwritten if both primary and secondary are IPv6. Signed-off-by: Usman Saleem <usman@usmans.info>
When the primary endpoint is IPv6 and ipv6Endpoint is also present, the ENR creation logic ignores ipv6Endpoint (since only dual-stack with IPv4 primary uses it). However, the validation logic was still checking if the ENR matched ipv6Endpoint, causing validation to always fail and the ENR to be unnecessarily recreated on every update. This fix aligns validation with creation by skipping ipv6Endpoint validation when primary is IPv6, preventing infinite ENR recreation. Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Usman Saleem <usman@usmans.info>
…ists Replace the comma-separated list approach for --p2p-host and --p2p-interface with dedicated IPv6-specific flags for better clarity and backward compatibility. Changes: - Replace --p2p-host (1..2 values) with: - --p2p-host (single value, IPv4 or IPv6) - --p2p-host-ipv6 (optional, IPv6 only) - Replace --p2p-interface (1..2 values) with: - --p2p-interface (single value) - --p2p-interface-ipv6 (optional, IPv6 only) - Removed classifyIpv4() and classifyIpv6() helper methods - Simplified toDomainObject() to use fields directly - Updated validation with dedicated methods for each option - Added IPv6-specific validation - Updated test config files to use new syntax Benefits: - Better backward compatibility (existing configs unchanged) - Explicit IPv6 configuration with clear opt-in - Cleaner validation with specific error messages - More intuitive API for users Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Usman Saleem <usman@usmans.info>
Decouples local binding configuration from outbound connection behavior by introducing a new CLI option --p2p-outbound-ip-version that controls which IP address to use when connecting to peers that advertise both IPv4 and IPv6. New Features: - CLI option: --p2p-outbound-ip-version (default: ipv4_preferred) - IpVersionPreference enum with 4 modes: * ipv4_preferred - prefer IPv4, fallback to IPv6 * ipv6_preferred - prefer IPv6, fallback to IPv4 * ipv4_only - only use IPv4 * ipv6_only - only use IPv6 Changes: - Created IpVersionPreference enum with smart validation - Updated DiscoveryPeerFactory to use enum instead of boolean - Updated PeerDiscoveryAgentV5 to use IpVersionPreference - Updated PeerDiscoveryAgentFactoryV5 to pass preference from config - Added outboundIpVersionPreference to DiscoveryConfiguration - Added CLI option in P2PDiscoveryOptions - Wired through P2PDiscoveryConfiguration and RunnerBuilder - Updated tests and test configuration files This allows operators to configure outbound connection preferences independently of their local binding configuration, enabling scenarios like listening on dual-stack but preferring IPv4 for compatibility, or forcing IPv4-only outbound when IPv6 routing is broken. Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Usman Saleem <usman@usmans.info>
Resolved merge conflicts in: - app/src/main/java/org/hyperledger/besu/RunnerBuilder.java - Kept both IpVersionPreference and ImmutableNetworkingConfiguration imports - ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/discv5/PeerDiscoveryAgentFactoryV5.java - Preserved IPv6 dual-stack logic with HostEndpoint objects - Removed redundant .listen() call that was replaced by conditional dual-stack logic Signed-off-by: Usman Saleem <usman@usmans.info>
Enforce that when dual-stack mode is enabled (--p2p-host-ipv6 or --p2p-interface-ipv6 is specified), the primary host/interface must be an IPv4 address. This prevents silent configuration errors where the IPv6 secondary endpoint would be ignored. Changes: - P2PDiscoveryOptions: Add validation to enforce IPv4 primary when IPv6 secondary is specified for both host and interface options - P2PDiscoveryOptionsTest: Add comprehensive test coverage for dual-stack validation scenarios - PeerDiscoveryAgentFactoryV5: Fix API calls after upstream merge (config.getDiscovery() -> config.discoveryConfiguration()) The validation ensures that: - IPv4-only configuration works as before - IPv6-only configuration works as before - Dual-stack requires IPv4 primary + IPv6 secondary - Clear error messages guide users to correct configuration Signed-off-by: Usman Saleem <usman@usmans.info>
Automatically set --p2p-interface-ipv6 to :: when --p2p-host-ipv6 is
specified without an explicit interface. This matches IPv4 behavior
where --p2p-interface defaults to 0.0.0.0 (listen on all interfaces).
Changes:
- P2PDiscoveryOptions: Add applySmartDefaults() method that:
* Auto-sets --p2p-interface-ipv6=:: when --p2p-host-ipv6 is specified
* Logs INFO message explaining the auto-configuration
* Warns when --p2p-interface-ipv6 is set without --p2p-host-ipv6
(incomplete dual-stack)
- P2PDiscoveryOptionsTest: Add 3 new tests:
* smartDefaultAppliesIpv6InterfaceWhenOnlyHostIpv6IsSpecified
* smartDefaultDoesNotOverrideExplicitIpv6Interface
* ipv4OnlyConfigurationDoesNotApplySmartDefault
Benefits:
- Reduces configuration verbosity - users only need to specify the
advertised IPv6 address to enable dual-stack
- Symmetric with IPv4 behavior - both default to listening on all
interfaces of their respective IP version
- Eliminates silent configuration failures where --p2p-host-ipv6
would be ignored without --p2p-interface-ipv6
Example usage before:
besu --p2p-host=192.0.2.1 \
--p2p-host-ipv6=2001:db8::1 \
--p2p-interface-ipv6=:: # Required but easy to forget
Example usage after:
besu --p2p-host=192.0.2.1 \
--p2p-host-ipv6=2001:db8::1 # Automatically enables dual-stack
Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Usman Saleem <usman@usmans.info>
- Remove redundant arity="1" from --p2p-host and --p2p-interface - Reorganize CLI options by IP family (IPv4/IPv6/Preference) - Convert --p2p-outbound-ip-version description to text block - Make port fields non-final (p2pPort, p2pPortIpv6) - Introduce DEFAULT_LISTENING_PORT_IPV6 constant (30404) - Replace magic number 30404 with constant across codebase Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Usman Saleem <usman@usmans.info>
Align casing (IPV6 -> IPv6), param case, and @return wording with existing methods in the class. Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Usman Saleem <usman@usmans.info>
…dress Signed-off-by: Usman Saleem <usman@usmans.info>
…nt opportunity Explain that the handler fires after ≥2 peers confirm our external address via PONG consensus, and why Optional.empty() is safe for IPv4 (NatService covers it) but leaves a gap for IPv6 where NatService does not apply and peer-observed addresses are the only auto-discovery path. Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Usman Saleem <usman@usmans.info>
6 tasks
Signed-off-by: Usman Saleem <usman@usmans.info>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+207
to
+220
| if (serverIpv6 != null) { | ||
| final CompletableFuture<Void> ipv6Close = new CompletableFuture<>(); | ||
| serverIpv6 | ||
| .channel() | ||
| .closeFuture() | ||
| .addListener( | ||
| future -> { | ||
| if (!future.isSuccess()) { | ||
| LOG.warn( | ||
| "Failed to close IPv6 RLPx socket cleanly: {}", future.cause().getMessage()); | ||
| } | ||
| ipv6Close.complete(null); // best-effort: don't block shutdown on IPv6 close failure | ||
| }); | ||
| CompletableFuture.allOf(ipv4Close, ipv6Close) |
There was a problem hiding this comment.
The IPv6 shutdown path waits on serverIpv6.channel().closeFuture() but does not initiate a close on the IPv6 channel in this method. If the IPv6 channel is not closed elsewhere, stop() can hang waiting for ipv6Close. Consider explicitly calling serverIpv6.channel().close() (and similarly ensure the IPv4 server channel is explicitly closed) before awaiting the close futures.
Comment on lines
+164
to
+167
| "Failed to bind IPv6 RLPx socket on {}:{}, continuing with IPv4 only: {}", | ||
| ipv6Host, | ||
| ipv6Port, | ||
| ipv6Future.cause().getMessage()); |
There was a problem hiding this comment.
This log line drops the throwable/stack trace by only logging cause().getMessage(). Consider passing the throwable itself as the final logger argument (or otherwise including it) so bind failures are diagnosable from logs.
Suggested change
| "Failed to bind IPv6 RLPx socket on {}:{}, continuing with IPv4 only: {}", | |
| ipv6Host, | |
| ipv6Port, | |
| ipv6Future.cause().getMessage()); | |
| "Failed to bind IPv6 RLPx socket on {}:{}, continuing with IPv4 only", | |
| ipv6Host, | |
| ipv6Port, | |
| ipv6Future.cause()); |
Comment on lines
+59
to
+61
| try (var conn = new Socket(addrs.ipv4Address().getAddress(), addrs.ipv4Address().getPort())) { | ||
| assertThat(conn.isConnected()).isTrue(); | ||
| } |
There was a problem hiding this comment.
Creating a Socket(host, port) can block for a long time if something goes wrong, which can make the test suite hang. Prefer creating an unconnected socket and using connect(SocketAddress, timeoutMillis) so failures are bounded and tests fail fast.
|
|
||
| initializer = createInitializer(dualStackConfig()); | ||
|
|
||
| final ListeningAddresses addrs = initializer.start().get(10, TimeUnit.SECONDS); |
There was a problem hiding this comment.
NettyConnectionInitializer.start() can intentionally degrade to IPv4-only (i.e., ipv6Address() empty) when the IPv6 bind fails, but this test unconditionally calls addrs.ipv6Address().get(). To avoid flaky failures on hosts where IPv6 is available but binding ::1 fails, add an assumption/assertion that ipv6Address() is present before accessing it (or otherwise handle the IPv4-only result).
Suggested change
| final ListeningAddresses addrs = initializer.start().get(10, TimeUnit.SECONDS); | |
| final ListeningAddresses addrs = initializer.start().get(10, TimeUnit.SECONDS); | |
| assumeTrue( | |
| addrs.ipv6Address().isPresent(), | |
| "IPv6 bind failed; initializer degraded to IPv4-only"); |
- Explicitly call channel.close() before awaiting closeFuture() on both IPv4 and IPv6 server channels to avoid potential shutdown hangs - Pass throwable to LOG.warn() for bind and close failures so stack traces are available in logs - Use Socket.connect(addr, timeout) in tests to bound connection time - Add assumeTrue(ipv6Address().isPresent()) guard in dual-stack tests to handle graceful IPv4-only degradation without test failures Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Usman Saleem <usman@usmans.info>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (!future.isSuccess()) { | ||
| LOG.warn("Failed to close IPv6 RLPx socket cleanly", future.cause()); | ||
| } | ||
| ipv6Close.complete(null); // best-effort: don't block shutdown on IPv6 close failure |
There was a problem hiding this comment.
The comment should use 'doesn't' instead of 'don't' for grammatical correctness.
Suggested change
| ipv6Close.complete(null); // best-effort: don't block shutdown on IPv6 close failure | |
| ipv6Close.complete(null); // best-effort: doesn't block shutdown on IPv6 close failure |
Comment on lines
+159
to
+170
| // Called when ≥2 peers independently report the same external address for this | ||
| // node via PONG responses (the discovery library requires multi-peer consensus | ||
| // before firing). The handler decides whether to update the local ENR with the | ||
| // peer-observed address. | ||
| // | ||
| // Currently returns Optional.empty() to ignore all peer feedback. This is safe | ||
| // for IPv4 because NatService (UPnP/NAT-PMP) handles external address discovery. | ||
| // For IPv6, however, NatService does not apply — peer-observed addresses would be | ||
| // the only auto-discovery mechanism. A future improvement could accept | ||
| // peer-suggested IPv6 addresses when --p2p-host-ipv6 is not explicitly configured, | ||
| // since IPv6 has no NAT and the peer-observed address is the real routable address. | ||
| // See: https://github.com/hyperledger/besu/issues/9874 |
There was a problem hiding this comment.
The comment explanation spans lines 159-170 but the actual implementation is a simple lambda returning Optional.empty(). Consider moving the detailed explanation to a dedicated method or condensing the inline comment to avoid overwhelming the builder chain with documentation.
Suggested change
| // Called when ≥2 peers independently report the same external address for this | |
| // node via PONG responses (the discovery library requires multi-peer consensus | |
| // before firing). The handler decides whether to update the local ENR with the | |
| // peer-observed address. | |
| // | |
| // Currently returns Optional.empty() to ignore all peer feedback. This is safe | |
| // for IPv4 because NatService (UPnP/NAT-PMP) handles external address discovery. | |
| // For IPv6, however, NatService does not apply — peer-observed addresses would be | |
| // the only auto-discovery mechanism. A future improvement could accept | |
| // peer-suggested IPv6 addresses when --p2p-host-ipv6 is not explicitly configured, | |
| // since IPv6 has no NAT and the peer-observed address is the real routable address. | |
| // See: https://github.com/hyperledger/besu/issues/9874 | |
| // Ignore peer-reported external addresses for now (always returns Optional.empty()). | |
| // For IPv4 this is covered by NatService; future IPv6 auto-discovery may relax this: | |
| // https://github.com/hyperledger/besu/issues/9874 |
- Declare serverIpv6 as volatile to ensure visibility between the Netty callback thread (writer) and the caller thread in stop() (reader) - Include bindHostIpv6/bindPortIpv6 in RlpxConfiguration equals(), hashCode(), and toString() so dual-stack configs are correctly distinguished and logged - Require both host and port for isDualStackEnabled() so a half-configured dual-stack is not silently treated as active; use orElseThrow() at the bind site to make the invariant explicit - Add test for graceful degradation: when the IPv6 bind fails (port pre-occupied), start() must still succeed with IPv4-only and return an empty ipv6Address() Signed-off-by: Usman Saleem <usman@usmans.info>
- Fix grammar: 'don't block' -> 'doesn't block' in IPv6 close comment - Condense verbose newAddressHandler comment to 3 lines referencing the tracking issue for IPv6 auto-discovery (besu-eth#9874) Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Usman Saleem <usman@usmans.info>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
191
to
205
| workers.shutdownGracefully(); | ||
| boss.shutdownGracefully(); | ||
|
|
||
| final CompletableFuture<Void> ipv4Close = new CompletableFuture<>(); | ||
| server | ||
| .channel() | ||
| .closeFuture() | ||
| .close() | ||
| .addListener( | ||
| (future) -> { | ||
| future -> { | ||
| if (future.isSuccess()) { | ||
| stoppedFuture.complete(null); | ||
| ipv4Close.complete(null); | ||
| } else { | ||
| stoppedFuture.completeExceptionally(future.cause()); | ||
| ipv4Close.completeExceptionally(future.cause()); | ||
| } | ||
| }); |
There was a problem hiding this comment.
The shutdown order is reversed: the event loop groups are shut down before closing the server channels. In Netty, closing channels should happen first, and then event loop groups should be shut down (optionally awaiting their termination). As written, shutdownGracefully() may prevent queued close tasks from executing promptly or reliably, risking hangs or incomplete shutdown. Consider: (1) close IPv4/IPv6 channels first, (2) compose both close futures, then (3) shutdown boss/workers and complete stop when both group terminations resolve.
Comment on lines
+140
to
+141
| // Both sockets are bound with ephemeral ports — they should differ | ||
| assertThat(addrs.ipv4Address().getPort()).isNotEqualTo(addrs.ipv6Address().get().getPort()); |
There was a problem hiding this comment.
This assertion is not guaranteed: many OSes allow the same port number to be bound simultaneously on different local IPs (e.g., 127.0.0.1:P and ::1:P), so two ephemeral binds can legitimately return the same port and make the test flaky. To validate 'independence', prefer asserting that each socket binds to its configured port (e.g., configure IPv6 to a specific known-free port while IPv4 remains ephemeral, then assert IPv6 uses the configured port), or remove/relax the inequality requirement.
Suggested change
| // Both sockets are bound with ephemeral ports — they should differ | |
| assertThat(addrs.ipv4Address().getPort()).isNotEqualTo(addrs.ipv6Address().get().getPort()); | |
| // Both sockets are bound successfully and have valid (non-zero) ports | |
| assertThat(addrs.ipv4Address().getPort()).isGreaterThan(0); | |
| assertThat(addrs.ipv6Address().get().getPort()).isGreaterThan(0); |
| * Holds the bound listening addresses after a successful {@link #start()}. | ||
| * | ||
| * @param ipv4Address the IPv4 socket address the RLPx server is bound to | ||
| * @param ipv6Address the IPv6 socket address, present only when dual-stack is active |
There was a problem hiding this comment.
The ipv6Address Javadoc is slightly inaccurate given the implementation: NettyConnectionInitializer returns Optional.empty() when dual-stack is configured but the IPv6 bind fails (graceful degradation). Update the doc to reflect 'present only when dual-stack is configured and the IPv6 bind succeeds' (or similar), so callers don’t assume presence solely from configuration.
Suggested change
| * @param ipv6Address the IPv6 socket address, present only when dual-stack is active | |
| * @param ipv6Address the IPv6 socket address, present only when dual-stack is configured and | |
| * the IPv6 bind succeeds |
Comment on lines
+412
to
+421
| // Include IPv6 ENR fields only when both an advertised IPv6 host is configured and an IPv6 | ||
| // RLPx socket was successfully bound. Omitting them when the socket is absent avoids | ||
| // advertising an incorrect tcp6 value to remote peers. | ||
| final Optional<HostEndpoint> ipv6Endpoint = | ||
| discoveryConfig | ||
| .getAdvertisedHostIpv6() | ||
| .flatMap( | ||
| host -> | ||
| ipv6TcpPort.map( | ||
| port -> new HostEndpoint(host, discoveryConfig.getBindPortIpv6(), port))); |
There was a problem hiding this comment.
This can advertise IPv6 ENR fields even when discovery itself is not listening on IPv6. HostEndpoint(host, discoveryConfig.getBindPortIpv6(), port) will set udp6 from the configured IPv6 discovery bind port regardless of whether discoveryConfig.isDualStackEnabled() (and thus whether builder.listen(ipv4, ipv6) is used). Consider additionally requiring discoveryConfig.isDualStackEnabled() (or otherwise confirming an IPv6 discovery socket is in use) before including the IPv6 HostEndpoint, so udp6/ip6 aren’t advertised incorrectly.
Suggested change
| // Include IPv6 ENR fields only when both an advertised IPv6 host is configured and an IPv6 | |
| // RLPx socket was successfully bound. Omitting them when the socket is absent avoids | |
| // advertising an incorrect tcp6 value to remote peers. | |
| final Optional<HostEndpoint> ipv6Endpoint = | |
| discoveryConfig | |
| .getAdvertisedHostIpv6() | |
| .flatMap( | |
| host -> | |
| ipv6TcpPort.map( | |
| port -> new HostEndpoint(host, discoveryConfig.getBindPortIpv6(), port))); | |
| // Include IPv6 ENR fields only when: | |
| // * an advertised IPv6 host is configured, | |
| // * an IPv6 RLPx socket was successfully bound, and | |
| // * discovery is running in dual-stack mode (i.e. an IPv6 discovery socket is in use). | |
| // Omitting them when any of these conditions is not met avoids advertising incorrect ip6/udp6 | |
| // or tcp6 values to remote peers. | |
| final Optional<HostEndpoint> ipv6Endpoint; | |
| if (discoveryConfig.isDualStackEnabled()) { | |
| ipv6Endpoint = | |
| discoveryConfig | |
| .getAdvertisedHostIpv6() | |
| .flatMap( | |
| host -> | |
| ipv6TcpPort.map( | |
| port -> | |
| new HostEndpoint(host, discoveryConfig.getBindPortIpv6(), port))); | |
| } else { | |
| ipv6Endpoint = Optional.empty(); | |
| } |
… ENR guards - NettyConnectionInitializer: close channels before shutting down event loop groups so channel-close callbacks have a running executor - NettyConnectionInitializerTest: replace flaky isNotEqualTo port assertion with isGreaterThan(0) — different IPs may receive the same ephemeral port from the OS - ConnectionInitializer.ListeningAddresses: clarify ipv6Address Javadoc to mention graceful-degradation (absent when IPv6 bind fails) - PeerDiscoveryAgentV5: guard IPv6 ENR fields behind isDualStackEnabled() to avoid advertising ip6/udp6/tcp6 without a live IPv6 UDP socket Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Usman Saleem <usman@usmans.info>
Matilda-Clerke
approved these changes
Feb 24, 2026
Contributor
Matilda-Clerke
left a comment
Matilda-Clerke
left a comment
There was a problem hiding this comment.
LGTM, just address the copilot comments before merging
Signed-off-by: Usman Saleem <usman@usmans.info>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR description
Two related improvements to the DiscV5 IPv6 dual-stack P2P networking stack:
Fix 1 — RLPx IPv6 dual-stack TCP binding
Previously, even when DiscV5 dual-stack discovery was active (two UDP sockets — one IPv4, one IPv6), RLPx only ever bound one TCP socket. IPv6-only peers that discovered us via the IPv6 discovery socket would fail to establish an RLPx connection if our primary TCP socket was IPv4-only.
RlpxConfigurationgainsbindHostIpv6,bindPortIpv6, andisDualStackEnabled()NettyConnectionInitializerbinds a secondServerBootstrapon the IPv6 address using the same shared Netty event loop groups. IPv6 bind failure degrades gracefully with a warning so IPv4 is never affected.ConnectionInitializer.start()returns aListeningAddressesrecord containing both bound addresses.RlpxAgentstores theListeningAddressesafter startup and exposesgetIpv6ListeningPort()for use by the discovery layer.RunnerBuildergates RLPx dual-stack on DiscV5 being enabled (--Xv5-discovery-enabled). IPv6 dual-stack support was introduced alongside DiscV5; Besu does not implement dual-stack for DiscV4, so binding a second TCP socket only makes sense when DiscV5 is active. This guard can be removed once DiscV4 is deprecated.Fix 2 — Fix DiscV5 ENR
tcp/tcp6port fieldsPeerDiscoveryAgentFactoryV5was eagerly building the localNodeRecordbefore the RLPx TCP port was known, causing the ENR'stcp/tcp6fields to always reflect the UDP discovery bind port instead of the actual RLPx listening port.Defer
NodeRecordinitialisation andDiscoverySystemconstruction toPeerDiscoveryAgentV5.start(tcpPort)where the effective ports are known:tcp= IPv4 RLPx port fromRlpxAgent.start()tcp6= IPv6 RLPx port fromRlpxAgent.getIpv6ListeningPort()— omitted when no IPv6 socket was boundLocalNodeKeySignermoved fromPeerDiscoveryAgentFactoryV5intoPeerDiscoveryAgentV5(co-located with its usage)DefaultP2PNetworksimplified: the conditionallisteningPortfallback inpeerDiscoveryAgent.start()is removed;listeningPortis now always passed directly since the discovery agent reads its own UDP port independentlywarnIfEphemeralPortsWithDiscV5()removed: the warning was premature; ephemeral UDP port support is a separate concern tracked in Consensys/discovery#198newAddressHandlercorrected: previously returnedOptional.of(nodeRecord)which accepted the external-address-update call but silently stored the unchanged record (a no-op). Now returnsOptional.empty()to properly reject external address suggestions from peers.Fixed Issue(s)
Part of #9686
Checklist
doc-change-requiredlabel if updates are required./gradlew spotlessApply./gradlew :ethereum:p2p:test